Conversation
Replaces gentropy's l2g_train_test_split, l2g_training and l2g_prediction with a single polars/XGBoost PTS step on one GCE VM. No Google Batch, no Dataproc, no Hugging Face Hub, no Weights & Biases, and only the pinned train/test split. Records the measurements the design rests on, including that shap caps the background at 100 samples so the current 1000-row sample is silently subsampled, that shapBaseValue already varies 1.75x across output partitions, that the OTG gold-standard branch is dead code on this data, and that the released model's hyperparameters (n_estimators=300, gamma=0, max_delta_step=1) are not in version control.
Nine TDD tasks: the feature contract, gold-standard parsing, the pinned split derivation, model fit/evaluate/persist, the SHAP process pool, the two transformer entry points with their config step, the orchestration retirement of the three gentropy steps, and a gated fixed-model parity harness against a published release. Also corrects the design's config sketch: the feature list is shared between the two tasks by a YAML anchor, not a scratchpad sentinel, because otter's Scratchpad is string.Template substitution and would deliver a list as its repr.
Seven findings from the pre-execution conflict scan, ruled and applied: - impute before cast reversed to cast before impute, matching gentropy's L2GFeatureMatrix, so the parity gate is not undermined by a rounding order - a Task 6 test asserted nothing (hasattr(pl, 'Path') is False, so the ternary always evaluated True); now asserts the artifact exists - pts_vep_view also depends on gentropy_l2g_prediction, which Task 8 never mentioned; repointing it to pts_l2g, without which the DAG cannot build - the empty-prediction test now pins whatever write_dataset does with a zero-row frame instead of assuming a part file exists - shapValue is null rather than NaN when explanations are off - a config test guards the YAML anchor against yamlfmt expanding it and letting the two tasks' feature lists drift - numpy imported normally rather than via __import__
xgboost>=3.4.1 and shap>=0.52.0 were an accident of the scratch benchmark venv, built on Python 3.12. uv forks both by Python version, so those floors would have forced pts off Python 3.11 and off its python:3.11-slim production base image for no functional gain. The real obstacle is narrower and unrelated to the Python floor: on x86_64 macOS only, shap requires numba<0.63, numba caps numpy<2.4, and pts requires numpy>=2.4.4. Excluding that single platform via tool.uv environments is the smallest fix, and unlike a linux+arm64 allowlist it keeps the Windows-only packages in the lock.
Add xgboost, shap, scikit-learn, and skops as pts dependencies, and the shared L2G feature contract (FEATURES, FIXED_COLUMNS, LOCUS_MEAN_IMPUTED, impute_and_cast) that later L2G training/prediction tasks build on. xgboost and shap are floored at 3.2.0/0.51.0 rather than 3.4.1/0.52.0: those newer floors only resolve on Python >=3.12, and pts stays on >=3.11 (production is python:3.11-slim). uv forks the two ranges by Python version, so this only changes what a >=3.12 dev environment would resolve to; production gets xgboost 3.2.0 / shap 0.51.0. Also add [tool.uv] environments, narrowed to exclude x86_64 macOS: shap's numba<0.63 pin collides with the baseline_expression extra's numpy>=2.4.4 requirement on that platform only, and is otherwise unsatisfiable. pts can no longer be locked/installed on an Intel Mac; production (linux) and local dev (Apple Silicon) are unaffected.
The 317 rows/s in the design was measured on an idle machine. Re-measured under concurrent load, on the same rows and the same background, it is 181 rows/s -- so the design overstated by ~1.7x and the table now carries the range with the slow end as the sizing basis. A controlled same-rows comparison isolates the library versions at ~6% (181 against 193 rows/s), so pinning production to xgboost 3.2.0 and shap 0.51.0 costs almost nothing. That pair also loads the released model and reproduces its published scores at max |delta| = 0. Conclusion is unchanged: under 5 vCPU-hours against the Batch job's envelope of up to 2,000, so a single VM remains the right shape. Also records that chunks must outnumber workers -- a chunk count below the worker count caps the speedup regardless of core count.
Per review: a per-file # isort: skip in model.py would have to be repeated in every future l2g module that touches both xgboost and skops, and a missed file reopens a SIGSEGV that names neither library. Import xgboost once in the package __init__ instead, so every submodule inherits the ordering.
The l2g/__init__.py guard only wins the race once that package is imported; a test module importing skops before ever touching pts.transformers.l2g still loses it. Import xgboost in tests/conftest.py, which pytest always loads before any test module in the directory, so the ordering holds regardless of collection order. Verified with a throwaway repro test (skops imported before the l2g package): segfaults without this guard, passes with it.
…test evaluate() computed averagePrecision from predict_proba scores; gentropy's L2GTrainer.evaluate computes it from the hard 0/1 predictions instead. The two differ by ~0.15 on the real 26.09-2 test split (0.781 vs 0.934), so a silent formula change would misread as a large model improvement against recorded runs. Report both: averagePrecision now matches gentropy exactly, and averagePrecisionFromScores carries the conventional (scores-based) form. Also strengthen test_fit_is_deterministic_for_a_fixed_seed: assert the fitted model actually reports random_state=777, since the prior bitwise- equality check alone would still pass even if random_state were dropped from DEFAULT_HYPERPARAMETERS.
Add a regression test that runs l2g_train twice (train_on_full_dataset True/False) on a fixture designed so a leak between the refit and evaluate() is unmissable -- the original shared fixture put every positive on genes the pinned test set also marked positive, so contamination stripped all positives out of training and made held-out metrics identical regardless of ordering. The new fixture gives train and test disjoint gene pools so training keeps real positives, and a leaked refit memorises the test rows outright (every metric hits 1.0) against an honest ~0.5-0.75 baseline. Also assert the two runs save different models, so the flag is not proven safe by being inert. Add test_config_l2g.py asserting config.yaml's l2g_train hyperparameters equal model.DEFAULT_HYPERPARAMETERS exactly, guarding the hand-duplicated block the same way the &l2g_features anchor guards the feature list.
Retires gentropy_l2g_train_test_split, gentropy_l2g_training and gentropy_l2g_prediction, including the 1000-task Google Batch job and its l2g_predict.sh entrypoint. gentropy_l2g_evidence now depends on pts_l2g.
In production `release_uri` is set, so otter resolves every relative destination in config.yaml into a `gs://…` URI before a transformer sees it. Three of l2g_train's writers were pathlib- or skops-backed, and POSIX collapses `gs://` to `gs:/`: the model, metrics.json and the split statistics landed under the container's working directory and the release shipped without them, silently, because l2g_predict then read the model back from the same collapsed local path. - `_write_json` and `save_model`/`load_model` route through otter's StorageHandle. - The background parquet drops its `Path(...).parent.mkdir` for `write_parquet(mkdir=True)`, the idiom release_metrics already uses for a single non-dataset release artifact. Also in this wave: - The gold standard's own `studyLocusId` is dropped before the join, as gentropy does. Kept, it arrived as `studyLocusId_right` and the dedup could not see past it, so two curated rows differing only in credible-set assignment became two training rows. Latent on today's curation file; the existing test could not see it because its two gold rows shared a studyLocusId. - The SHAP pool pins the `spawn` start method. The default is spawn on macOS but fork on Linux, and production is python:3.11-slim; forking after predict_proba and TreeExplainer have run their OpenMP regions is the classic hang, with no execution_timeout to bound it. Every test of this path has run under spawn, so this makes production run the exercised configuration. - The parity gate calls l2g_predict instead of re-implementing its preparation. Deleting the isProteinCoding filter, or changing the threshold comparison, previously left it passing. - OUTPUT_COLUMNS is enforced by a final select rather than merely documented; the config features_list is checked against FEATURES, not only against its own copy; and three docstrings quoting superseded measurements are corrected against the design spec.
l2g_training was the only consumer of wandb-key and it has moved into pts, which does not use Weights & Biases. Nothing else references W&B, so every gentropy cluster was fetching a secret no step reads -- and would have failed to create at all once the secret was retired. HF_TOKEN stays: hfhub-key is still fetched by operators/gce.py.
…pread The full-scale gate against do/platform-2609-1, using that release's own model over its own feature matrix across all 200 partitions, reproduced every one of the 3,219,816 published rows with max |delta| 1.192e-07 and none above the 1e-6 tolerance, and matched the threshold row set exactly. Corrects a third sampled figure. The design said shapBaseValue varies 1.75x across partitions, from six sampled parts. Across all 200 there are 200 distinct values spanning 0.028042 to 0.137713, a 4.91x spread. The argument for a single seeded background is stronger than the design claimed, not weaker.
…annot see The rewritten parity gate catches a deleted isProteinCoding filter, but it cannot catch the threshold comparison being flipped from `>=` to `>`: no released row scores exactly 0.05 in float64, so both produce identical output and no gate over released data could tell them apart. The module docstring implied otherwise. - The parity docstring now states precisely which mutations the gate catches and which it cannot, and points at the unit test that covers the boundary. - That unit test derives its threshold from a score the model actually produced -- score everything, take a real score, re-run with it as the threshold -- so the boundary row and the threshold are the same number and no floating-point engineering is needed. Verified to fail under `>`: "the row scoring exactly at the threshold was dropped".
polars joins leave their output order unspecified, and `derive_splits` ends in one, so the same inputs came back in a different row order on every run -- three orders from three calls in a single process. Both things downstream are positional: XGBoost's `subsample=0.8` draws its rows by position, and `build_background`'s `rng.choice` draws the SHAP background by position. The fitted model and `shapBaseValue` therefore differed between runs while `metrics.json` recorded `shapBackgroundSeed` as though the draw were reproducible. Sort train and test on `(studyLocusId, geneId)` -- the natural key, unique in this data -- immediately after the split, which is where the positional dependency begins. `labelled` inherits the order through `pl.concat`. The new test runs `l2g_train` twice from one fixture and requires the two saved models to score identically and the two `shap_background.parquet` files to be equal. Without the sort it fails on both halves: the backgrounds differ, and the models score the test split 0.03907016 against 0.04322654.
`shap_background_size` was set twice, once under `transform l2g_train` and again under `transform l2g_predict`, with nothing binding the two. Raise the training one to 1000 and forget the other and `Independent(max_samples=100)` would silently use 100 of the 1000 rows while `metrics.json` recorded 1000 -- the same truncated-background defect this migration exists to remove. `features_list` is held together by a YAML anchor and `hyperparameters` by a test, and this had neither. The masker is now sized as `max_samples=background.shape[0]`, from the parquet training wrote, and the setting is gone from the predict task. A config test asserts it stays gone. Also from the review: * the `OUTPUT_COLUMNS` re-select claimed a reordering would raise. It is silently corrected instead, and only a rename or a drop raises. Reworded. * `build_output` publishes the imputed feature value where gentropy re-joins the raw matrix and would publish a null. Recorded as deliberate -- the value should be the one its `shapValue` explains -- and unreachable here, since all 31 features are null-free across all 200 partitions of the real matrix. * `test_l2g_predict_output_is_sorted_by_key` asserted only `studyLocusId` on a fixture with one gene per locus, so sorting by that alone would have passed. It now runs on twelve loci of four genes each, staged in descending key order, and compares the key tuples. * `test_l2g_predict_applies_the_threshold` accepted either outcome behind `if parts:`. The behaviour is determinate, `pl.PartitionBy` writing one readable zero-row `00000000.parquet`, so assert that and drop the branch.
Three documents contradicted the shipped code. * The design's training walkthrough said `evaluate` reports "the same six metrics gentropy reports". It reports seven. `averagePrecision` keeps gentropy's hard-label formula so earlier runs stay comparable, and `averagePrecisionFromScores` is the conventional form. They differ by roughly 0.15 on the real test split, so the two are now named and distinguished. * `explain.py` and `test_l2g_parity.py` quoted the published `shapBaseValue` spread as 0.0381 to 0.0668, a 1.75x range from a six-partition sample. The design already retracted it. Measured across all 200 partitions of `do/platform-2609-1` it is 200 distinct values from 0.028042 to 0.137713, a 4.91x spread. The design's own acceptance section still repeated the old figure and now agrees with itself. The parity test's `> 1.5` assertion is left alone. It still holds and a tighter one would be calibrated to a single release, which is now said in place. `test_features_has_31_names_in_fitted_order` checked the count, the uniqueness and the two ends, so any transposition among the 29 names between them passed. It now asserts the whole tuple against an explicit literal. `CURATED_COLUMNS` omits `traitFromSourceMappedId`, which gentropy's `L2GGoldStandard` schema allows. A future curated file carrying it would have it dropped, and two rows differing only there would collapse in `annotate`'s `unique`. Documented, along with why gentropy's `n_unique_positive_gene_disease_pairs` statistic is absent for the same reason. Behaviour unchanged.
It was stale in every place review changed something: all four instances of the artifact-loss defect, the gold-standard join that duplicates training rows, the process pool that forks on Linux, the chunk size that serialises the pool, a six-key metrics assertion where seven are reported, and code that sorts nothing before two positional operations determining the model. Anyone implementing from it would have reproduced the lot. Its job was to drive the implementation and that is done. The design document stays and is current. The plan survives outside the repo as the record of what was planned, annotated with the nine places the shipped code deliberately diverges from it.
The sort added in 2f155d5 is a total order only while `(studyLocusId, geneId)` is unique. Polars' `sort` is not tie-stable, and `maintain_order=True` would not close the gap, because it stabilises against the input order and the input order is precisely what the joins in `derive_splits` leave unspecified. A repeated pair would therefore put the tied rows back in an arbitrary relative order and hand the positional draws below -- `subsample` and the SHAP background -- back their run-to-run variation. The failure would be invisible. `metrics.json` would still record its seeds, `test_l2g_train_is_reproducible_across_runs` would still pass on its key-unique fixture, and the only symptom would be a different model every release with nothing to say why. `require_unique_key` now raises a `ValueError` after the sort, naming the count and up to five of the offending pairs. The new test feeds a gold standard carrying two curated rows that agree on `(studyId, variantId, geneId)` and disagree on `goldStandardSet`, which is the one way a pair survives `annotate`'s whole-row `unique` twice. The duplicate is placed on a locus that survives the contamination anti-join, so it lands in the train split rather than being filtered away first. Without the check the test fails with `DID NOT RAISE ValueError`.
Follows the plan out. Both were working documents for the migration rather than something this repository needs to carry: `docs/superpowers/` is a tooling convention, and the repo has no other root-level docs tree. The design survives outside the repo as the single current copy, with the parity results and the corrections made during review. The PR description carries what a reviewer needs to judge the change without it.
Measured on the real 26.09-2 inputs rather than estimated: 23m12s against n1-highmem-32's 39m40s, a 1.71x improvement that matches a per-core benchmark across five machine families. n2d and c3 cores are ~1.6x faster than n1 at interventional TreeSHAP, which is 98% of this step. Output is unchanged: the split statistics still reproduce all fourteen of that release's recorded numbers, and output/l2g_prediction comes to the same 564,562,707 bytes. Keeps a memory-generous shape deliberately. Peak RSS is 28.2 GB, transient, during the polars scan and join of the 5.5 GB of parquet inputs -- SHAP itself is a flat ~12.7 GB. A 32 GB highcpu shape was tried and thrashed, with under 4 GB left for the OS, page cache and the spawning workers. The comment records this so the step is not "right-sized" into failing.
Measured, not estimated: 16m06s against n2d-standard-32's 23m12s and n1-highmem-32's 39m40s, on the real 26.09-2 inputs. The earlier choice of 32 was reasoned from a ratio without its absolute. 64 cores is 1.44x faster for 1.39x the core-time, which sounds like a poor trade until you notice the difference is about 4.8 vCPU-hours per run. Seven minutes of wall clock is worth that. The step is memory-bandwidth-bound, so this is close to the end of what scaling up can buy: utilisation drops from 81% to 67% between the two shapes and total CPU consumed rises 15.6% for identical work. The comment records that, so the next person does not spend a day discovering it again. Output is equivalent: 3,295,999 rows, one shapBaseValue, correct key ordering, and all fourteen split statistics still exact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
Prototype, opened to aid discussion about the L2G step logic — not a merge candidate.
It is complete, reviewed and verified against a real release, but it changes how a published
dataset is produced. Treat it as a concrete proposal to argue with.
Moves L2G training and prediction out of
gentropyand intopts.What changes
gentropy_l2g_train_test_split(Dataproc)gentropy_l2g_training(Dataproc)pts_l2gstep, two transformer tasks, one GCE VMgentropy_l2g_prediction(Google Batch, 200 tasks)polars, XGBoost and shap instead of PySpark. No Dataproc, no Batch, no Hugging Face Hub, no
Weights & Biases, and only the pinned train/test split — no cross-validation, no re-split.
gentropy_l2g_feature_matrixstays put.gentropy_l2g_evidenceandpts_vep_viewnow depend onpts_l2g. The step lands on a plain VM because PTS routes to Dataproc only when a task is namedpyspark …; both tasks here aretransform ….Limitations of the previous implementation
Each of these is a property of the old design rather than bad luck on one run, and each is closed
here. The figures come from the Batch job history, the release output, and this project's billing
export — not from estimates.
1. The published
shapBaseValuewas not a single numberA SHAP base value is the model's expected output over the background distribution — one model, one
background, one number. In release 26.09-2 the published predictions carry 200 distinct
shapBaseValues, from 0.028042 to 0.137713, a 4.91× spread, because each Batch task drew its ownunseeded 100-row background from whatever slice of the data it happened to hold. Every
shapValuein the release is an offset from a base its neighbours do not share, so the per-row explanations are
not comparable across the dataset, and re-running gentropy on identical inputs would produce
different numbers again.
Fixed. One background is drawn once, from a seeded sample, and shared by the whole run. Verified
on the full-scale run:
distinct shapBaseValue: 1. This also means row-level SHAP parity with therelease was never achievable — not by this port, and not by gentropy re-running itself.
2. Producing 26.09-2 took eight Batch submissions and 16 hours
Every one of the 200 tasks independently called
LocusToGeneModel.load_from_huband pulled themodel over the public internet from huggingface.co. Any single task that failed to reach the Hub
killed the entire 200-task job, and because
max_retry_countonly covers Batch's reserved50001-50005 exit codes, an application exit 1 never retried. The full record for the 26.09-2 run
(
manual__2026-09-07T15:32:59, every job carryingstep=gentropy_l2g_prediction):8 submissions, 1 useful. The Hub error hit 5 of the 8 jobs and 21 task instances — the
distinct-task counts above come from
batch_task_logs, and only the two marked FAILED show it intheir job status, because a task that errors is not always the task that ends the job. 108 minutes
of job runtime of which 27 were productive — 75% wasted, 544 of 722 vCPU-hours, £20.43 of the
£27.10 the step actually cost. First submission to final success was 16h13m; both hard
failures landed at 20:02 and 20:38 and were not recovered until the next morning, so the step
blocked the release overnight.
Nothing was fixed in between. The successful attempt is the one where huggingface.co happened to
answer 200 times in a row — the failure mode is still there, in the released code, waiting on
someone else's uptime.
Three of those cancellations are a second, independent defect: an ending Airflow task does not stop
the Batch job it launched, so clearing a task that still owned a live job submitted another beside
it. At 10:46 three jobs — 600 tasks — were writing the same output partitions concurrently with
Spark
overwritesemantics, and the prefix had to be wiped and redone because the output could notbe vouched for.
For balance: 26.09-1 and the 08-29 run each succeeded on their first submission. The point is not
that the step always failed, but that when it failed it took a person, a night, and a manual cleanup
to recover, and nothing in its design prevented any of that. The new step has no Hugging Face
call, no fan-out, and no external network dependency at all — the model is read from the release
bucket beside its own predictions.
3. Every task read the whole feature matrix
Each of the 200 Batch tasks started its own Spark job on two vCPUs and read the entire 2.5 GB
feature matrix — only the credible-set side was partitioned — to emit ~3,200 rows. Measured across
the three successful production runs of
up-gentropy-l2g-prediction:pts_l2g)n1-standard-2n2d-standard-641.7× faster on 10.6× less compute, at 1/12th the cost — 1/47th against what 26.09-2 actually
cost. Costs are derived from this project's own billing export (N1 £0.0256/core-h +
£0.00319/GB-h; N2D £0.0223/core-h + £0.00278/GB-h, EMEA, on demand); they cover the compute only
and exclude the Dataproc clusters the two removed training steps also needed.
Earlier drafts of this description said "an envelope of up to 2,000 vCPU-hours". That was the
configured ceiling (
max_task_count: 1000against a one-hour cap), not anything that happened. Themeasured figure is 181.
4. It could not be run outside Google Cloud
Three steps, two of them on Dataproc and one on Batch, with a Hugging Face Hub round trip in the
middle. The step now runs end to end on a laptop (M-series MacBook, inputs streamed from GCS):
That is the whole training half and the scoring half of the step, on real release inputs, on a
laptop. SHAP is the one part that stays a server job — it is 98% of the step's compute, and the
laptop figure above deliberately excludes it rather than extrapolating one.
Verification against release 26.09-2
Measured against
do/platform-2609-1anddo/platform-2609-2, never fixtures.Prediction — the gated harness (
L2G_PARITY_RUN=do/platform-2609-1), 3 passed in 12m05s. Itruns the release's own
classifier.skopsthrough the shippedl2g_predictover the release'sown feature matrix, all 200 partitions:
max |Δ| = 1.192e-07, zero rows above the 1e-6 toleranceTraining — which that gate cannot reach, since it holds the model constant:
n_train 80872,n_test_new 18011,n_lost_test 177, …), including from a full run on a VM against cloud storageImportant
Any published ~0.93 L2G metric is refit-on-everything, not held-out. gentropy sets
train_on_full_dataset: true, so the releasedclassifier.skopsis refit on train + test;scoring it on that test split scores a model on its own training data. This step refits the same
way for the same reason, so its saved model also scores ~0.93 there — but its reported
heldOutblock is computed before the refit, and is ~0.77.There is therefore no published gentropy figure to compare honest held-out performance
against. The evidence that training is faithful is the fourteen exact split statistics, not a
metric comparison. An earlier draft of this description compared 0.9337 to 0.9337 and called that
fidelity; both numbers were leaked and the comparison was meaningless.
Performance
SHAP is 98% of the step. Machine shape was chosen by benchmarking five families at 8 vCPU with the
released model and real rows, then confirming each candidate at full size:
n1-highmem-32(first choice)n2d-standard-32n2d-standard-64(shipped)2.5× faster than the first shape that worked, with equivalent output at every shape — 3,295,999
rows, one
shapBaseValue, correct key ordering, split statistics exact.n1-highmem-32was thenaive like-for-like replacement for the Batch fleet, and on it the step would have been slower
than the 27m12s it replaced; the win in the table above is a machine-shape result as much as an
architectural one.
Two things worth knowing before tuning this further:
fell 81% → 67% while total CPU consumed rose 15.6% for identical work — the extra cores stall on
memory, and stalling counts as busy. 128 cores would buy little. The only large win left is
algorithmic:
pred_contribsis ~200× faster but emits log-odds margin contributions rather thanprobability-space SHAP values, changing the published
shapValuenumbers.inputs; SHAP itself sits at a flat ~12.7 GB. A 32 GB
highcpushape was tried and thrashed —silently, with no OOM kill. That peak grows with input size, so the generous memory is deliberate.
Memory also scales with
shap_workers, because the pool spawns rather than forks.What this uncovered, beyond the port
classifier.skopscarriesn_estimators=300,gamma=0,max_delta_step=1; gentropy's config has none of them, andXGBoost's default gives 100 rounds. They are now explicit in
pts/config.yaml, guarded by a test.shapsilently caps a background at 100 samples, so gentropy'ssample(n=1_000)has alwaysbeen a 100-row background. The masker is now constructed explicitly.
averagePrecisionwas computed on hard labels, not scores. Kept under that name forcomparability with existing history, with
averagePrecisionFromScoresalongside; they differ by~0.15 on real data.
data. Deliberately not ported; a file in that format now fails loudly.
Review
Nine per-task reviews, then a whole-branch review, then three independent reviewers (Claude, Codex
gpt-5.5, CodeRabbit) run in parallel and told nothing of earlier findings. Two defects emergedthat per-task review structurally could not see.
Artifact loss (Critical).
l2g_trainwrote the model,metrics.jsonand the split statisticswith
pathlib/skops, which cannot addressgs://. In production those three landed on thecontainer's local disk under a collapsed
gs:/path while the step reported success — and the nexttask read the model back from that same local path, so nothing failed. A release would have shipped
correct predictions with no model beside them. Every test used local destinations, where the bug is
invisible. Fixed, and verified against a real bucket.
Non-reproducibility (Important). polars joins leave output order unspecified, and
.unique(maintain_order=True)preserves that unspecified order. Since XGBoost'ssubsampleand theSHAP background draw are both positional, two runs on byte-identical inputs produced different
models — while
metrics.jsonrecorded a seed implying reproducibility. Verified: three runs of arepresentative join chain in one process gave three different row orders. Fixed by sorting on
(studyLocusId, geneId)before anything positional, with a guard that refuses a split whose keyrepeats, since the sort is only a total order when the key is unique.
Also fixed:
shap_background_sizewas set twice inconfig.yamlwith nothing binding the two, soraising one and forgetting the other would silently restore the original 100-row truncation; three
tests asserted less than their names promised; several docstrings quoted superseded figures.
Two findings were deliberately not acted on, and are documented in the code instead. gentropy
builds the released
features[].valuepayload from the raw matrix while this port carries theimputed values through — unreachable here (zero nulls across all 31 features in all 200
partitions), and ours is the more coherent of the two, since gentropy would publish a value that
does not correspond to its own SHAP value. And
traitFromSourceMappedIdis silently dropped, whichis latent for a future curated file.
Known limitations
execution_timeout, so a hang here would run until someone noticed. Thefork/OpenMP hazard that motivated this is closed — Linux was confirmed to default to
fork, thepool now pins
mp_context='spawn', and a full Linux run succeeded — but the timeout gap is realand pre-existing.
sio.dumpsbuffers the serialised model in memory. Immaterial at 300 trees.pl.concat(how='horizontal')emits a DeprecationWarning. Deliberately left: the frames areequal-height by construction, so the call is already correct under the announced future semantics,
and
horizontal_extendwould move it the wrong way.docs/superpowers/isa tooling convention rather than something this repository carries. Everything needed to judge the
change is in this description.